Skip to content

Update Python versions#43

Merged
tacaswell merged 4 commits into
matplotlib:masterfrom
hugovk:update-versions
Dec 21, 2017
Merged

Update Python versions#43
tacaswell merged 4 commits into
matplotlib:masterfrom
hugovk:update-versions

Conversation

@hugovk

@hugovk hugovk commented Dec 15, 2017

Copy link
Copy Markdown
Contributor

And add python_requires to help pip.

Latest master fails on the old 2.7 on Appveyor: https://ci.appveyor.com/project/hugovk/cycler/build/1.0.1

@Kojoley

Kojoley commented Dec 15, 2017

Copy link
Copy Markdown
Member

Is PYTHON_VERSION and PYTHON_ARCH used anywhere?

@hugovk hugovk mentioned this pull request Dec 15, 2017
@hugovk

hugovk commented Dec 15, 2017

Copy link
Copy Markdown
Contributor Author

@Kojoley

Kojoley commented Dec 15, 2017

Copy link
Copy Markdown
Member

You are right... then a path you defined for 3.6 interfere with appveyor installed python.

@hugovk

hugovk commented Dec 15, 2017

Copy link
Copy Markdown
Contributor Author

Which line interferes?

At least AppVeyor passes: https://ci.appveyor.com/project/hugovk/cycler/build/1.0.4

By the way, should AppVeyor be enabled for this repo? Then you get builds for PRs on AppVeyor and Travis.

@Kojoley

Kojoley commented Dec 15, 2017

Copy link
Copy Markdown
Member

Which line interferes?

Appveyor has installed python under C:\Python36-x64 path https://www.appveyor.com/docs/build-environment/#python.
Actually I do not know why we are downloading and installing the python. It looks unnecessary to me.

By the way, should AppVeyor be enabled for this repo?

@tacaswell could you please enable appveyor on organization account for cycler? It looks like I can't do it myself.

@hugovk

hugovk commented Dec 15, 2017

Copy link
Copy Markdown
Contributor Author

I'm not sure Why Python is downloaded rather then using the existing versions, but I bet there was a good reason. Maybe a specific version was needed. Does @tacaswell know?

Should we try it out with the system Pythons?

@tacaswell tacaswell closed this Dec 15, 2017
@tacaswell tacaswell reopened this Dec 15, 2017
@codecov-io

codecov-io commented Dec 15, 2017

Copy link
Copy Markdown

Codecov Report

Merging #43 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #43   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         177    177           
  Branches       50     50           
=====================================
  Hits          177    177

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eaa0f63...5dc55d8. Read the comment docs.

@tacaswell

Copy link
Copy Markdown
Member

power-cycled to see if appveyor will fire.

Those old build failures look like dependency issues (missing setuptools) rather than cycler not working with old versions of 2.7.

@tacaswell

Copy link
Copy Markdown
Member

ohh, sorry, not reading the urls clearly (@hugovk 's account above and the emails I got from appveyor were tied to my personal account not matplotlib's). I'll see if I can sort this out when not on tethered-to-phone internet.

@Kojoley

Kojoley commented Dec 15, 2017

Copy link
Copy Markdown
Member

Those old build failures look like dependency issues (missing setuptools) rather than cycler not working with old versions of 2.7.

Yeah, cycler uses python setup.py install, so setuptools should have been installed before calling it.

@hugovk

hugovk commented Dec 20, 2017

Copy link
Copy Markdown
Contributor Author

Is there anything I can do to move this closer to merge?

@tacaswell

Copy link
Copy Markdown
Member

https://ci.appveyor.com/project/matplotlib/cycler appveyor should be active now

@tacaswell tacaswell closed this Dec 20, 2017
@tacaswell tacaswell reopened this Dec 20, 2017
@tacaswell

Copy link
Copy Markdown
Member

'power cycled' again to trigger appveyor.

No good reason to not use the system python. This appevyor configuration file was cargo-cult-ed from matplotlib/matplotlib which needs to compile c/c++. It can probably be greatly simplified.

@Kojoley

Kojoley commented Dec 20, 2017

Copy link
Copy Markdown
Member

@hugovk I have rebased and forcepushed to your branch.

@tacaswell

Copy link
Copy Markdown
Member

I am quite happy that this is a -250 line PR

@tacaswell tacaswell merged commit 60f4da3 into matplotlib:master Dec 21, 2017
@tacaswell

Copy link
Copy Markdown
Member

Thanks @Kojoley and @hugovk !

@hugovk hugovk deleted the update-versions branch December 21, 2017 06:40
@QuLogic QuLogic added this to the v1.0 milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants